vNext: Add migration step for publication-dissertation - #920
Conversation
| ### Fix thesis resource type | ||
|
|
||
| 1. Update the resource type in `vocabularies/resource_types.yaml` by updating the label for `publication-dissertation` to "Thesis" and then run: | ||
| ```python | ||
| invenio rdm-records add-to-fixture resourcetypes | ||
| ``` | ||
|
|
||
| 2. Run the data migration script in [#data-migration](#data-migration) section. | ||
|
|
||
| 3. Delete `publication-thesis` vocabulary: | ||
|
|
||
| ```sh | ||
| from invenio_vocabularies.proxies import current_service as vocabulary_service | ||
| vocabulary_service.delete(system_identity, ('resourcetypes', 'publication-thesis')) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
The easiest way to convey my comments was provide a re-write 😅 . I will explain a couple points here:
- Not specific to this: The upgrade-vNext.md needs to be restructured overall, so the hierarchy here might change.
- This migration step will not be used by a number of instances since it depends on the instance using those exact resource types (
publication-{dissertation/thesis}) and wanting to switch - there is no explainer why one would want to switch too though. (we won't use it at all for instance, because we don't use those exact resource type ids). As such the script in invenio-app-rdm should be separated out from the script with "required" operations (see name below). This is work to do in invenio-app-rdm. Once that is done we can merge this with any tweaks you want to apply to the suggestions here. - Not specific to this, but in general all the steps in the upgrade doc are a bit hard to follow to understand if they are applicable, so I am suggesting a little "does this migration step apply to me" header to help readers.
- I did verify the add-to-fixture parts and that's ok
| ### Fix thesis resource type | |
| 1. Update the resource type in `vocabularies/resource_types.yaml` by updating the label for `publication-dissertation` to "Thesis" and then run: | |
| ```python | |
| invenio rdm-records add-to-fixture resourcetypes | |
| ``` | |
| 2. Run the data migration script in [#data-migration](#data-migration) section. | |
| 3. Delete `publication-thesis` vocabulary: | |
| ```sh | |
| from invenio_vocabularies.proxies import current_service as vocabulary_service | |
| vocabulary_service.delete(system_identity, ('resourcetypes', 'publication-thesis')) | |
| ``` | |
| ### Change resource type `publication-thesis` to `publication-dissertation` | |
| **Does this migration step apply to me?**: | |
| *Yes*: You are using `publication-thesis` as a resource type (e.g., because you are using the default resource types) AND you want to use `publication-dissertation` instead. | |
| *No*: Otherwise. | |
| **Yes sub-steps** | |
| 1. If you are using a customized list of resource types in `<my_instance>/app_data/vocabularies/resource_types.yaml`, then: | |
| - set `title.<lang>` to "Thesis" (in appropriate language) for entry with `id` equal to `publication-dissertation` | |
| - remove the entry with `id` equal to `publication-thesis` | |
| If you didn't customize resource types, you can skip this step. | |
| 2. Apply the resource types title change. | |
| - `invenio rdm-records add-to-fixture resourcetypes` | |
| - Note that this will change the title, but will not delete the `publication-thesis` from your data stores. Deletion is done in step 4. | |
| 3. Run the publication dissertation migration script: | |
| ```shell | |
| invenio shell $(find $(dirname $(dirname $(uv python find)))/lib/*/site-packages/invenio_app_rdm -name migrate_13_0_to_14_publication_dissertation.py) | |
| ``` | |
| 4. Delete `publication-thesis` vocabulary via invenio shell: | |
| ```python | |
| from invenio_vocabularies.proxies import current_service as vocabulary_service | |
| vocabulary_service.delete(system_identity, ('resourcetypes', 'publication-thesis')) | |
| ``` | |
There was a problem hiding this comment.
Thank you for the suggestion
I have updated the docs and also added a PR for updating the upgrade script: inveniosoftware/invenio-app-rdm#3501
Please review @fenekku @utnapischtim @slint
There was a problem hiding this comment.
I am looking into this today :)
d6639ff to
17c609b
Compare
|
Sorry if I have seen this a bit too late. I think that this migration should be mandatory, transparent and done during the upgrade. The only thing we can do with the script is to check if thesis and dissertation exist in the system. If yes, we should apply it. We can provide the above explanation in the upgrade doc. If not, then we should skip it. To be discussed. |
|
Just to be clear, we're talking about forcing the migration from the resource type Frankly, in this particular instance, I disagree with having a forced migration of the values. In our instance, we have a bunch of records that do refer to Bachelor's or Master's theses specifically [1], not a PhD dissertation. In my opinion, this migration should not be mandatory, but it should be left to the instance operator to decide whether or not they want to migrate existing records to the new value.
I can see scenarios where instance operators might have customized the resource types beforehand already, and have explicitly decided on [1] e.g. https://researchdata.tuwien.ac.at/records/rjqr7-0b076 To be clear, if we were setting up a fresh instance, that'd be a very different question for us; then we might roll with the default value (and perhaps add our own custom value), but I don't intend to migrate our existing records forcibly. |
|
DataCite metadata working group already had a considerable amount of discussions on this topic, and they came up with: This change changes the internal ID, but keeps the text |
|
I see and understand that, but I still think that with all the possibilities that InvenioRDM offers and actually advertises regarding customization of controlled vocabularies, I believe that the instances' data is not ours to forcibly change anymore. Of course, the possibility of skipping should be based on informed consent and can also be on an opt-out basis. |
|
I would like a prominent warning in the update docs, too! Our librarians are discussing a lot, too, and not always comply with the results from Datacite discussions. I would even say, they rarely do. |
something like that: https://github.com/inveniosoftware/invenio-app-rdm/pull/3501/changes which would be an opt in |
|
Haha I am glad this is getting discussed! A quick summary from my end: The situation as I understand it Use to be
(eurepo just added to show surrounding semantic whether right or wrong...) These are both "technically" datacite compliant (use available datacite_general values from the controlled list), but publication-thesis was deemed not "semantically" compliant: it was "a duplication that no one noticed", because there was already publication-dissertation. publication-thesis was seen as pretty much the same thing as publication-dissertation and Datacite says to use datacite_general Text only if no other value is applicable (and Dissertation was applicable in a sense). So publication-thesis was removed from the fixture and code was added to the migration script to align/convert records+drafts with resource type id: publication-thesis to id: publication-dissertation. The problem with forcing that migration as I see it I think Max put it into words better than I did. Because instances may have customized their resource types, this migration is potentially fragile and invasive:
All that to say that, in general, when these highly visible and commonly customized vocabularies are changed, my thinking is that we should err on suggesting rather than imposing the changes. The flag in that PR works for me! Providing a separate script the migrator may want to run is fine too. "Whatever a framework does for you, it does to you" and all that. Thanks! |
|
Using a custom Text resource type should be avoided whenever possible. It makes interoperability between more difficult. This situation will happen again in the future when DataCite will include a new type in the supported resource types that we have now mapped to a custom Text. And this is happening and it will happen with other metadata fields. I understand the customizations of resource types, but this should be handled by the script. I don't see any technical limitation. If we simply propose or suggest it, I am pretty convinced no one will ever do it. This will require a discussion on a call. It would be preferable to also have @tmorrell if he is available (and any other interested person). |
|
I'm not suggesting a mere warning, I'm suggesting an informed decision point, and like I've said, I'm also fine with an opt-out. And in case the user explicitly selects "no" (the default being "yes"), print some information on how they can do this in the future. Could also be prominently documented in the upgrade guide, but I think the script would be a better place.
InvenioRDM is interoperable via API, but instances are generally not interoperable on a per-entry basis. Regarding metadata schema, from my understanding we're only "heavily inspired" by DataCite but don't follow it 100% either. |
|
I'd also vote for making the migration optional. Resource types are really specific to the institution, and I think there are valid use cases to having both resource types. I like @max-moser 's suggestion of having an opt-out as part of the upgrade script. |
|
@fenekku @max-moser I have added a new commit to explain the why. |
|
v14 cleanup Per maintainer meeting, a section dedicated to the optional publication-dissertation migration will be added at the end of the |
ff5c3f2 to
a10b7b9
Compare
|
Ping @fenekku, I've pushed the changes we've discussed and you've summarized in #920 (comment). If you can have a quick look and make sure the placement and wording are fine, feel free also to apply any small changes directly and merge. I know you're making a full pass as well, so I'm leaving it up to you to make your merge/rebase life easier 😅 |
fenekku
left a comment
There was a problem hiding this comment.
Thanks for that! Mostly comments suggesting rewording the "standard"/"custom" mentions.
The code for the migration is fine as a one-off (didn't go deep in it to be honest).
Good to go after!
a10b7b9 to
3a3412b
Compare
3a3412b to
41bc15c
Compare
|
@fenekku thanks for all the suggestions, I've applied and pushed. Good to merge now :) |
related: inveniosoftware/invenio-app-rdm#3186